Skip to content

fix(io): order image-directory frames naturally - #558

Open
AlexBodner wants to merge 1 commit into
developfrom
fix/video-natural-frame-order
Open

fix(io): order image-directory frames naturally#558
AlexBodner wants to merge 1 commit into
developfrom
fix/video-natural-frame-order

Conversation

@AlexBodner

@AlexBodner AlexBodner commented Aug 12, 2026

Copy link
Copy Markdown
Collaborator

Problem

_iter_image_folder_frames sorts directory entries lexicographically, so 1.jpg … 10.jpg is read as 1, 10, 11, 2, 3, … Frames are then numbered sequentially by enumeration, so a temporally shuffled sequence still gets sequential frame ids.

Nothing raises. The tracker receives frames out of order, motion between consecutive frames is nonsense, and every track is garbage — with no indication why.

When this happens

Only through frames_from_source on an image directory — the public API and trackers track <dir>. The trigger is unpadded numeric filenames, which is what ffmpeg -i video.mp4 %d.jpg produces.

Not reported by anyone, and benchmarking can't hit it: MOT Challenge and DanceTrack frames are zero-padded (6- and 8-digit), and the eval path resolves frames by index through resolve_mot_frame_path rather than sorting a listing. But io/frames.py already falls back to a plain-int stem — "Try both, then plain int" — so unpadded naming is a layout the library expects to meet.

Fix

Sort on a natural key that compares embedded digit runs as integers. Zero-padded and non-numeric names sort identically under both keys, so benchmark sequences are unaffected.

Mixed-width numeric names get no warning: natural ordering resolves them correctly, so a warning would fire on valid input.

Tests

  • Unpadded 1.png11.png read in numeric order — fails before the change, where frame 2 returned the contents of 10.png.
  • Non-numeric names keep alphabetical order.
  • Zero-padded names still work. Pre-existing test, renamed from test_reads_images_in_alphabetical_order since the contract is no longer alphabetical.

tests/io + tests/cli: 341 passed.

`_iter_image_folder_frames` sorted entries lexicographically, so a
directory of unpadded names was read as 1, 10, 11, 2, 3, ... The frames
were then numbered sequentially, so a temporally shuffled sequence
produced sequential frame ids and nonsense tracks with no error raised.

Sorting on a natural key restores numeric order for unpadded names.
Zero-padded and non-numeric names sort identically under both keys, so
benchmark sequences are unaffected.

No warning is emitted for mixed-width numeric names: natural ordering
resolves them correctly, so a warning would fire on valid input.

Co-authored-by: Cursor <cursoragent@cursor.com>
@Borda
Borda requested a balanced review from Copilot August 12, 2026 18:46

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds natural filename ordering for image-directory frame sources.

Changes:

  • Sorts embedded numeric filename segments numerically.
  • Documents ordering behavior.
  • Adds numeric, alphabetic, and zero-padded ordering tests.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
src/trackers/io/video.py Implements and documents natural sorting.
tests/io/test_video.py Tests image-directory ordering behavior.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread tests/io/test_video.py
Comment on lines +179 to +186
for index, stem in enumerate(("alpha", "beta", "gamma")):
cv2.imwrite(str(directory / f"{stem}.png"), create_frame(index))

frames = list(frames_from_source(directory))

assert len(frames) == 3
for frame_id, frame in frames:
assert np.all(frame == expected_frame_value(frame_id - 1))
@AlexBodner
AlexBodner marked this pull request as ready for review August 18, 2026 22:03
@AlexBodner
AlexBodner requested a review from SkalskiP as a code owner August 18, 2026 22:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants